Skip to content

fix: remove loadSourceForBridge in RCTRootViewFactory#43656

Closed
okwasniewski wants to merge 1 commit into
facebook:mainfrom
okwasniewski:fix/rootviewfactory-bridgedlegate
Closed

fix: remove loadSourceForBridge in RCTRootViewFactory#43656
okwasniewski wants to merge 1 commit into
facebook:mainfrom
okwasniewski:fix/rootviewfactory-bridgedlegate

Conversation

@okwasniewski

Copy link
Copy Markdown
Contributor

Summary:

This PR removes forward declaration of loadSourceForBridge methods from the RCTRootViewFactory as it was causing an issue on old architecture, where the RedBox wouldn't popup when metro wasn't running.

As stated by @Kudo here:

the problem was coming from the implementation

if ([self.delegate respondsToSelector:@selector(loadSourceForBridge:onProgress:onComplete:)]) {
[self.delegate loadSourceForBridge:_parentBridge onProgress:onProgress onComplete:onSourceLoad];
} else if ([self.delegate respondsToSelector:@selector(loadSourceForBridge:withBlock:)]) {
[self.delegate loadSourceForBridge:_parentBridge withBlock:onSourceLoad];
} else if (!self.bundleURL) {
NSError *error = RCTErrorWithMessage(
@"No bundle URL present.\n\nMake sure you're running a packager "
"server or have included a .jsbundle file in your application bundle.");
onSourceLoad(error, nil);
} else {
__weak RCTCxxBridge *weakSelf = self;
[RCTJavaScriptLoader loadBundleAtURL:self.bundleURL
onProgress:onProgress
onComplete:^(NSError *error, RCTSource *source) {
if (error) {
[weakSelf handleError:error];
return;
}
onSourceLoad(error, source);
}];
}
}

we should dynamically override loadSourceForBridge:onProgress:onComplete: and loadSourceForBridge:withBlock: in RCTRootViewFactory only when AppDelegate override it. one way to achieve this might be tricky that we may need to override respondsToSelector:.

otherwise, we could just skip the loadSourceForBridge:onProgress:onComplete: and loadSourceForBridge:withBlock: support.

There is no straight forward solution to implement this without some hacks so I'm removing this forward block for now.

Changelog:

[IOS] [FIXED] - remove loadSourceForBridge in RCTRootViewFactory

Test Plan:

CI Green

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Callstack Partner: Callstack Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Mar 26, 2024
@okwasniewski okwasniewski force-pushed the fix/rootviewfactory-bridgedlegate branch from 8ae6b6c to a3796e7 Compare March 26, 2024 14:42
@okwasniewski okwasniewski force-pushed the fix/rootviewfactory-bridgedlegate branch from a3796e7 to 0204379 Compare March 28, 2024 14:10
@analysis-bot

Copy link
Copy Markdown
Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 18,483,534 +16
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 21,856,615 -7
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 41a9664
Branch: main

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@okwasniewski

Copy link
Copy Markdown
Contributor Author

Hey, can you guys let me know what the issues are on internal linter? I'll fix them

@cortinico

Copy link
Copy Markdown
Contributor

Hey, can you guys let me know what the issues are on internal linter? I'll fix them

Sorry I was out due to public holidays. I've fixed it for you, we should be able to merge it soon

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Apr 2, 2024
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@cortinico merged this pull request in 0c02b26.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Callstack Partner: Callstack Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants